Skip to content

DI: capture expressions#5845

Merged
TonyCTHsu merged 46 commits into
masterfrom
di-capture-expressions
Jul 14, 2026
Merged

DI: capture expressions#5845
TonyCTHsu merged 46 commits into
masterfrom
di-capture-expressions

Conversation

@p-datadog

@p-datadog p-datadog commented Jun 1, 2026

Copy link
Copy Markdown
Member

What does this PR do?

Adds capture-expression support to Ruby log probes in Dynamic Instrumentation. A capture expression is a named DSL expression attached to a log probe; at probe-fire time, the expression is evaluated against the probe scope and its serialized value is emitted in the snapshot payload under captures.…​.captureExpressions.<name>. Capture expressions are an alternative to captureSnapshot: true: they let the user select exactly which values to capture instead of the whole local/argument scope.

Motivation:

Feature parity with other languages.

Limitations:

Method probes (still) do not capture locals, therefore it is not possible to capture method locals via capture expressions also.
Positional arguments are (still) reported as arg1 etc. to the backend and users must specify that to capture them. Works but unintuitive.

Change log entry

Yes. Dynamic Instrumentation log probes now support capture expressions as an alternative to capturing the full local/argument scope.

Manual Verification

UI:
2026-07-01_21-00

Line probe:
2026-07-01_20-59

Method probe - positional arg:
2026-07-01_21-36

Method probe - keyword arg:
2026-07-01_21-24

@p-datadog p-datadog added the AI Generated Largely based on code generated by an AI or LLM. This label is the same across all dd-trace-* repos label Jun 1, 2026
@dd-octo-sts dd-octo-sts Bot added core Involves Datadog core libraries debugger Live Debugger (+Dynamic Instrumentation, +Symbol Database) labels Jun 1, 2026
@dd-octo-sts

dd-octo-sts Bot commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Typing analysis

Note: Ignored files are excluded from the next sections.

steep:ignore comments

This PR introduces 15 steep:ignore comments, and clears 15 steep:ignore comments.

steep:ignore comments (+15-15)Introduced:
lib/datadog/di/instrumenter.rb:190
lib/datadog/di/instrumenter.rb:195
lib/datadog/di/instrumenter.rb:199
lib/datadog/di/instrumenter.rb:204
lib/datadog/di/instrumenter.rb:208
lib/datadog/di/instrumenter.rb:227
lib/datadog/di/instrumenter.rb:288
lib/datadog/di/instrumenter.rb:290
lib/datadog/di/instrumenter.rb:893
lib/datadog/di/probe_builder.rb:26
lib/datadog/di/probe_notification_builder.rb:180
lib/datadog/di/probe_notification_builder.rb:416
lib/datadog/di/serializer.rb:257
lib/datadog/di/serializer.rb:389
lib/datadog/di/serializer.rb:504
Cleared:
lib/datadog/di/instrumenter.rb:183
lib/datadog/di/instrumenter.rb:188
lib/datadog/di/instrumenter.rb:192
lib/datadog/di/instrumenter.rb:197
lib/datadog/di/instrumenter.rb:201
lib/datadog/di/instrumenter.rb:220
lib/datadog/di/instrumenter.rb:281
lib/datadog/di/instrumenter.rb:283
lib/datadog/di/instrumenter.rb:862
lib/datadog/di/probe_builder.rb:24
lib/datadog/di/probe_notification_builder.rb:139
lib/datadog/di/probe_notification_builder.rb:375
lib/datadog/di/serializer.rb:248
lib/datadog/di/serializer.rb:377
lib/datadog/di/serializer.rb:492

Untyped methods

This PR introduces 2 untyped methods and 27 partially typed methods, and clears 2 untyped methods and 31 partially typed methods. It increases the percentage of typed methods from 66.0% to 66.39% (+0.39%).

Untyped methods (+2-2)Introduced:
sig/datadog/di/probe_notification_builder.rbs:58
└── def tags: () -> untyped
sig/datadog/di/probe_notification_builder.rbs:60
└── def serialized_tags: () -> untyped
Cleared:
sig/datadog/di/probe_notification_builder.rbs:52
└── def tags: () -> untyped
sig/datadog/di/probe_notification_builder.rbs:54
└── def serialized_tags: () -> untyped
Partially typed methods (+27-31)Introduced:
sig/datadog/di/context.rbs:29
└── def initialize: (probe: Probe, settings: Datadog::Core::Configuration::Settings, serializer: Serializer, ?locals: Hash[Symbol, untyped]?, ?target_self: untyped?, ?path: String?, ?caller_locations: Array[Thread::Backtrace::Location]?, ?serialized_entry_args: Hash[Symbol, untyped]?, ?entry_capture_expressions: Hash[String, any]?, ?entry_capture_evaluation_errors: Array[Hash[Symbol, String]]?, ?return_value: untyped?, ?duration: Float?, ?exception: Exception?) -> void
sig/datadog/di/context.rbs:50
└── def serialized_locals: () -> Hash[Symbol, untyped]?
sig/datadog/di/context.rbs:52
└── def fetch: ((Symbol | String) var_name) -> untyped
sig/datadog/di/context.rbs:54
└── def fetch_ivar: ((Symbol | String) var_name) -> untyped
sig/datadog/di/instrumenter.rbs:42
└── def hook_method: (Probe probe, untyped responder) -> void
sig/datadog/di/instrumenter.rbs:45
└── def hook_line: (Probe probe, untyped responder) -> bool?
sig/datadog/di/instrumenter.rbs:49
└── def hook: (Probe probe, untyped responder) -> void
sig/datadog/di/instrumenter.rbs:53
└── def self.get_local_variables: (TracePoint trace_point) -> Hash[Symbol, untyped]
sig/datadog/di/instrumenter.rbs:54
└── def self.get_instance_variables: (Object self) -> Hash[Symbol, untyped]
sig/datadog/di/instrumenter.rbs:61
└── def run_method_probe: (::Array[untyped] args, ::Hash[::Symbol, untyped] kwargs, ::Proc? target_block, untyped target_self, Probe probe, untyped responder, [::String, ::Integer]? loc, ::String method_name) { () -> untyped } -> untyped
sig/datadog/di/instrumenter.rbs:65
└── def kwargs_from_splat: (::Array[untyped] args) -> [::Array[untyped], ::Hash[untyped, untyped]]
sig/datadog/di/instrumenter.rbs:71
└── def line_trace_point_callback: (Probe probe, RubyVM::InstructionSequence? iseq, untyped responder, TracePoint tp) -> void
sig/datadog/di/instrumenter.rbs:75
└── def check_and_disable_if_exceeded: (Probe probe, untyped responder, Float di_start_time, ?Float accumulated_duration) -> void
sig/datadog/di/probe_builder.rbs:12
└── def self?.build_template_segments: (Array[Hash[String, untyped]]? segments) -> Array[String | DI::EL::Expression]?
sig/datadog/di/probe_notification_builder.rbs:22
└── def build_received: (Probe probe) -> Hash[Symbol,untyped]
sig/datadog/di/probe_notification_builder.rbs:24
└── def build_installed: (Probe probe) -> Hash[Symbol,untyped]
sig/datadog/di/probe_notification_builder.rbs:26
└── def build_emitting: (Probe probe) -> Hash[Symbol,untyped]
sig/datadog/di/probe_notification_builder.rbs:28
└── def build_errored: (Probe probe, Exception exception) -> Hash[Symbol,untyped]
sig/datadog/di/probe_notification_builder.rbs:30
└── def build_disabled: (Probe probe, Float duration) -> Hash[Symbol,untyped]
sig/datadog/di/probe_notification_builder.rbs:32
└── def build_executed: (Context context) -> Hash[Symbol,untyped]
sig/datadog/di/probe_notification_builder.rbs:34
└── def build_snapshot: (Context context) -> Hash[Symbol,untyped]
sig/datadog/di/probe_notification_builder.rbs:39
└── def build_snapshot_base: (Context context, ?evaluation_errors: Array[Hash[Symbol, String]]?, ?captures: Hash[Symbol, untyped]?, ?message: String?) -> Hash[Symbol,untyped]
sig/datadog/di/probe_notification_builder.rbs:41
└── def build_condition_evaluation_failed: (Context context, EL::Expression expr, Exception exception) -> Hash[Symbol,untyped]
sig/datadog/di/probe_notification_builder.rbs:43
└── def build_status: (Probe probe, message: String, status: String, ?exception: Exception?) -> Hash[Symbol,untyped]
sig/datadog/di/probe_notification_builder.rbs:45
└── def format_caller_locations: (Array[Thread::Backtrace::Location] callers) -> Array[Hash[Symbol,untyped]]
sig/datadog/di/probe_notification_builder.rbs:49
└── def tag_process_tags!: (Hash[Symbol | String, untyped] payload, Core::Configuration::Settings settings) -> void
sig/datadog/di/probe_notification_builder.rbs:53
└── def get_local_variables: (TracePoint trace_point) -> Hash[Symbol,untyped]
Cleared:
sig/datadog/di/context.rbs:26
└── def initialize: (probe: Probe, settings: Datadog::Core::Configuration::Settings, serializer: Serializer, ?locals: Hash[Symbol, untyped]?, ?target_self: untyped?, ?path: String?, ?caller_locations: Array[Thread::Backtrace::Location]?, ?serialized_entry_args: Hash[Symbol, untyped]?, ?return_value: untyped?, ?duration: Float?, ?exception: Exception?) -> void
sig/datadog/di/context.rbs:45
└── def serialized_locals: () -> Hash[Symbol, untyped]?
sig/datadog/di/context.rbs:47
└── def fetch: ((Symbol | String) var_name) -> untyped
sig/datadog/di/context.rbs:49
└── def fetch_ivar: ((Symbol | String) var_name) -> untyped
sig/datadog/di/instrumenter.rbs:38
└── def hook_method: (Probe probe, untyped responder) -> void
sig/datadog/di/instrumenter.rbs:41
└── def hook_line: (Probe probe, untyped responder) -> bool?
sig/datadog/di/instrumenter.rbs:45
└── def hook: (Probe probe, untyped responder) -> void
sig/datadog/di/instrumenter.rbs:49
└── def self.get_local_variables: (TracePoint trace_point) -> Hash[Symbol, untyped]
sig/datadog/di/instrumenter.rbs:50
└── def self.get_instance_variables: (Object self) -> Hash[Symbol, untyped]
sig/datadog/di/instrumenter.rbs:57
└── def run_method_probe: (::Array[untyped] args, ::Hash[::Symbol, untyped] kwargs, ::Proc? target_block, untyped target_self, Probe probe, untyped responder, [::String, ::Integer]? loc, ::String method_name) { () -> untyped } -> untyped
sig/datadog/di/instrumenter.rbs:61
└── def kwargs_from_splat: (::Array[untyped] args) -> [::Array[untyped], ::Hash[untyped, untyped]]
sig/datadog/di/instrumenter.rbs:67
└── def line_trace_point_callback: (Probe probe, RubyVM::InstructionSequence? iseq, untyped responder, TracePoint tp) -> void
sig/datadog/di/instrumenter.rbs:71
└── def check_and_disable_if_exceeded: (Probe probe, untyped responder, Float di_start_time, ?Float accumulated_duration) -> void
sig/datadog/di/probe_builder.rbs:6
└── def self?.build_from_remote_config: (Hash[String, untyped] config) -> Probe
sig/datadog/di/probe_builder.rbs:8
└── def self?.build_template_segments: (Array[Hash[String, untyped]]? segments) -> Array[String | DI::EL::Expression]?
sig/datadog/di/probe_notification_builder.rbs:16
└── def build_received: (Probe probe) -> Hash[Symbol,untyped]
sig/datadog/di/probe_notification_builder.rbs:18
└── def build_installed: (Probe probe) -> Hash[Symbol,untyped]
sig/datadog/di/probe_notification_builder.rbs:20
└── def build_emitting: (Probe probe) -> Hash[Symbol,untyped]
sig/datadog/di/probe_notification_builder.rbs:22
└── def build_errored: (Probe probe, Exception exception) -> Hash[Symbol,untyped]
sig/datadog/di/probe_notification_builder.rbs:24
└── def build_disabled: (Probe probe, Float duration) -> Hash[Symbol,untyped]
sig/datadog/di/probe_notification_builder.rbs:26
└── def build_executed: (Context context) -> Hash[Symbol,untyped]
sig/datadog/di/probe_notification_builder.rbs:28
└── def build_snapshot: (Context context) -> Hash[Symbol,untyped]
sig/datadog/di/probe_notification_builder.rbs:33
└── def build_snapshot_base: (Context context, ?evaluation_errors: Array[Hash[Symbol, String]]?, ?captures: Hash[Symbol, untyped]?, ?message: String?) -> Hash[Symbol,untyped]
sig/datadog/di/probe_notification_builder.rbs:35
└── def build_condition_evaluation_failed: (Context context, EL::Expression expr, Exception exception) -> Hash[Symbol,untyped]
sig/datadog/di/probe_notification_builder.rbs:37
└── def build_status: (Probe probe, message: String, status: String, ?exception: Exception?) -> Hash[Symbol,untyped]
sig/datadog/di/probe_notification_builder.rbs:39
└── def format_caller_locations: (Array[Thread::Backtrace::Location] callers) -> Array[Hash[Symbol,untyped]]
sig/datadog/di/probe_notification_builder.rbs:43
└── def tag_process_tags!: (Hash[Symbol | String, untyped] payload, Core::Configuration::Settings settings) -> void
sig/datadog/di/probe_notification_builder.rbs:47
└── def get_local_variables: (TracePoint trace_point) -> Hash[Symbol,untyped]
sig/datadog/di/serializer.rbs:25
└── def serialize_args: (Array[untyped] args, Hash[Symbol, untyped] kwargs, untyped instance_vars, ?depth: Integer, ?attribute_count: Integer?) -> Hash[Symbol, untyped]
sig/datadog/di/serializer.rbs:26
└── def serialize_vars: (Hash[Symbol, untyped] vars, ?depth: Integer, ?attribute_count: Integer?) -> Hash[Symbol, untyped]
sig/datadog/di/serializer.rbs:27
└── def serialize_value: (untyped value, ?name: (Symbol | String)?, ?depth: Integer, ?attribute_count: Integer?, ?type: Class?) -> Hash[Symbol, untyped]

Untyped other declarations

This PR introduces 3 untyped other declarations and 2 partially typed other declarations, and clears 3 untyped other declarations and 2 partially typed other declarations. It increases the percentage of typed other declarations from 83.7% to 84.05% (+0.35%).

Untyped other declarations (+3-3)Introduced:
sig/datadog/di/context.rbs:23
└── @return_value: untyped
sig/datadog/di/context.rbs:39
└── attr_reader target_self: untyped
sig/datadog/di/context.rbs:46
└── attr_reader return_value: untyped
Cleared:
sig/datadog/di/context.rbs:20
└── @return_value: untyped
sig/datadog/di/context.rbs:36
└── attr_reader target_self: untyped
sig/datadog/di/context.rbs:41
└── attr_reader return_value: untyped
Partially typed other declarations (+2-2)Introduced:
sig/datadog/di/context.rbs:37
└── attr_reader locals: Hash[Symbol, untyped]?
sig/datadog/di/context.rbs:43
└── attr_reader serialized_entry_args: Hash[Symbol, untyped]?
Cleared:
sig/datadog/di/context.rbs:34
└── attr_reader locals: Hash[Symbol, untyped]?
sig/datadog/di/context.rbs:40
└── attr_reader serialized_entry_args: Hash[Symbol, untyped]?

If you believe a method or an attribute is rightfully untyped or partially typed, you can add # untyped:accept on the line before the definition to remove it from the stats.

@datadog-datadog-prod-us1

datadog-datadog-prod-us1 Bot commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Tests

🎉 All green!

🧪 All tests passed
❄️ No new flaky tests detected

🎯 Code Coverage (details)
Patch Coverage: 35.68%
Overall Coverage: 89.88% (-0.15%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 60e8c77 | Docs | Datadog PR Page | Give us feedback!

@pr-commenter

pr-commenter Bot commented Jun 1, 2026

Copy link
Copy Markdown

Benchmarks

Benchmark execution time: 2026-07-14 18:04:19

Comparing candidate commit 60e8c77 in PR branch di-capture-expressions with baseline commit 4e01cc7 in branch master.

📊 Benchmarking dashboard

Found 0 performance improvements and 0 performance regressions! Performance is the same for 48 metrics, 1 unstable metrics.

Explanation

This is an A/B test comparing a candidate commit's performance against that of a baseline commit. Performance changes are noted in the tables below as:

  • 🟩 = significantly better candidate vs. baseline
  • 🟥 = significantly worse candidate vs. baseline

We compute a confidence interval (CI) over the relative difference of means between metrics from the candidate and baseline commits, considering the baseline as the reference.

If the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD), the change is considered significant.

Feel free to reach out to #apm-benchmarking-platform on Slack if you have any questions.

More details about the CI and significant changes

You can imagine this CI as a range of values that is likely to contain the true difference of means between the candidate and baseline commits.

CIs of the difference of means are often centered around 0%, because often changes are not that big:

---------------------------------(------|---^--------)-------------------------------->
                              -0.6%    0%  0.3%     +1.2%
                                 |          |        |
         lower bound of the CI --'          |        |
sample mean (center of the CI) -------------'        |
         upper bound of the CI ----------------------'

As described above, a change is considered significant if the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD).

For instance, for an execution time metric, this confidence interval indicates a significantly worse performance:

----------------------------------------|---------|---(---------^---------)---------->
                                       0%        1%  1.3%      2.2%      3.1%
                                                  |   |         |         |
       significant impact threshold --------------'   |         |         |
                      lower bound of CI --------------'         |         |
       sample mean (center of the CI) --------------------------'         |
                      upper bound of CI ----------------------------------'

Unstable benchmarks

These benchmarks have a confidence interval too wide to call a change; treat them as noise rather than signal.

scenario:tracing - trace.to_digest - Continue

  • unstable throughput [-1445.876op/s; +1685.418op/s] or [-4.996%; +5.823%]

@p-datadog p-datadog requested a review from Copilot June 2, 2026 20:17
@p-datadog

Copy link
Copy Markdown
Member Author

@codex review

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds Dynamic Instrumentation “capture expressions” support for Ruby log probes, enabling selective value capture (via DSL expressions) into snapshot payloads under captureExpressions, with configurable per-fire serialization time budgeting.

Changes:

  • Parse captureExpressions (including per-expression capture limits) from remote config and attach them to Datadog::DI::Probe.
  • Evaluate capture expressions at probe fire time with a configurable time budget and merge per-expression evaluation errors into snapshot evaluationErrors.
  • Extend serializer plumbing to allow per-evaluation overrides for string length / collection size limits; add docs, RBS signatures, and specs.

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
supported-configurations.json Adds env var metadata for max serialization time budget.
spec/datadog/di/probe_spec.rb Adds unit coverage for probe capture_expressions behavior and rate-limit defaults.
spec/datadog/di/probe_builder_spec.rb Adds remote-config parsing tests for captureExpressions and related behaviors.
spec/datadog/di/capture_expression_spec.rb Adds tests for capture expression / limits value objects and limit resolution behavior.
spec/datadog/di/capture_expression_evaluator_spec.rb Adds tests for evaluation success/failure, timeout behavior, and limit propagation to serializer.
sig/datadog/di/serializer.rbs Updates serialize_value signature to include length and collection_size.
sig/datadog/di/probe.rbs Adds capture-expression-related fields and predicate signature.
sig/datadog/di/probe_notification_builder.rbs Adds capture_expression_evaluator ivar/reader typing.
sig/datadog/di/probe_builder.rbs Adds capture-expression builder helper typings and name pattern constant.
sig/datadog/di/capture_expression.rbs Adds RBS for CaptureExpression and CaptureLimits.
sig/datadog/di/capture_expression_evaluator.rbs Adds RBS for evaluator class and return types.
lib/datadog/di/serializer.rb Threads per-evaluation length/collection_size through recursive serialization.
lib/datadog/di/probe.rb Stores capture-expression config on probes and adjusts default rate limiting.
lib/datadog/di/probe_notification_builder.rb Evaluates capture expressions for snapshots and merges evaluation errors.
lib/datadog/di/probe_builder.rb Parses captureExpressions from remote config and validates names/expr structure.
lib/datadog/di/configuration.rb Introduces max_time_to_serialize_ms DI setting backed by env var.
lib/datadog/di/capture_expression.rb Adds CaptureExpression and CaptureLimits classes plus limit-resolution logic.
lib/datadog/di/capture_expression_evaluator.rb Implements evaluator with time budget, serialization, and error reporting.
lib/datadog/di/boot.rb Ensures capture-expression components are loaded during DI boot.
lib/datadog/core/configuration/supported_configurations.rb Registers new env var in supported configuration list (generated).
docs/DynamicInstrumentation.md Documents capture expressions, behavior choices, and new env var.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/datadog/di/capture_expression.rb Outdated
Comment thread lib/datadog/di/probe_builder.rb

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5affe59f77

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread lib/datadog/di/capture_expression.rb Outdated
Comment thread lib/datadog/di/probe_notification_builder.rb Outdated
p-datadog pushed a commit that referenced this pull request Jun 2, 2026
…d length

The CaptureLimits.resolve doc comment promises a per-field fallback chain
of expr_limits -> probe -> settings, but for collection_size and length the
code went straight from the expression-level override to settings, skipping
the probe-level overrides parsed into probe.max_capture_collection_size /
probe.max_capture_string_length. That meant a probe-level capture.maxCollectionSize
or capture.maxLength from remote config was silently ignored unless repeated
on every expression.

Adds the probe-level step to both fallback chains so the resolver matches
its documented contract. The serializer already accepts length: and collection_size:
keyword arguments, so the fix flows end-to-end.

Addresses review comments on PR #5845 from @Copilot (3344166066) and
@chatgpt-codex-connector (3344167297).

Co-Authored-By: Claude <noreply@anthropic.com>
p-datadog pushed a commit that referenced this pull request Jun 2, 2026
build_capture_expressions called raw.empty? before verifying raw was an
Array. A non-Array value from remote config (e.g. an Integer) would raise
NoMethodError instead of the intended ArgumentError, and the outer
rescue KeyError in build_from_remote_config would not wrap it either —
producing a misleading error class even though the bare rescue in
Component#parse_probe_spec_and_notify still kept it from escaping.

Reorders the early-return: nil short-circuits, then the Array type check,
then the empty-array short-circuit. Adds a probe_builder_spec test for
the non-Array case.

Addresses review comment on PR #5845 from @Copilot (3344166098).

Co-Authored-By: Claude <noreply@anthropic.com>
p-datadog pushed a commit that referenced this pull request Jun 2, 2026
The return-time Context for method probes was built with locals: nil, so
capture expressions referencing arg1 or kwarg names resolved as
undefined/nil — the condition path at instrumenter.rb:135 already populates
locals via serializer.combine_args(args, kwargs, self) but the return-time
path didn't.

Passes combine_args(args, kwargs, self) as locals when the probe has
capture expressions; keeps nil otherwise to avoid an allocation on every
method invocation for probes that don't need it. At return time the args
reference is the same Ruby object as at entry, so values reflect any
in-place mutation by the method body — matching what conditions already
see and what cross-tracer convention reports for method exit scope.

Adds an integration test exercising a method probe with a capture
expression referencing arg1 against a positional argument.

Addresses review comment on PR #5845 from @chatgpt-codex-connector (3344167304).

Co-Authored-By: Claude <noreply@anthropic.com>
p-datadog pushed a commit that referenced this pull request Jun 9, 2026
…d length

The CaptureLimits.resolve doc comment promises a per-field fallback chain
of expr_limits -> probe -> settings, but for collection_size and length the
code went straight from the expression-level override to settings, skipping
the probe-level overrides parsed into probe.max_capture_collection_size /
probe.max_capture_string_length. That meant a probe-level capture.maxCollectionSize
or capture.maxLength from remote config was silently ignored unless repeated
on every expression.

Adds the probe-level step to both fallback chains so the resolver matches
its documented contract. The serializer already accepts length: and collection_size:
keyword arguments, so the fix flows end-to-end.

Addresses review comments on PR #5845 from @Copilot (3344166066) and
@chatgpt-codex-connector (3344167297).

Co-Authored-By: Claude <noreply@anthropic.com>
p-datadog pushed a commit that referenced this pull request Jun 9, 2026
build_capture_expressions called raw.empty? before verifying raw was an
Array. A non-Array value from remote config (e.g. an Integer) would raise
NoMethodError instead of the intended ArgumentError, and the outer
rescue KeyError in build_from_remote_config would not wrap it either —
producing a misleading error class even though the bare rescue in
Component#parse_probe_spec_and_notify still kept it from escaping.

Reorders the early-return: nil short-circuits, then the Array type check,
then the empty-array short-circuit. Adds a probe_builder_spec test for
the non-Array case.

Addresses review comment on PR #5845 from @Copilot (3344166098).

Co-Authored-By: Claude <noreply@anthropic.com>
p-datadog pushed a commit that referenced this pull request Jun 9, 2026
The return-time Context for method probes was built with locals: nil, so
capture expressions referencing arg1 or kwarg names resolved as
undefined/nil — the condition path at instrumenter.rb:135 already populates
locals via serializer.combine_args(args, kwargs, self) but the return-time
path didn't.

Passes combine_args(args, kwargs, self) as locals when the probe has
capture expressions; keeps nil otherwise to avoid an allocation on every
method invocation for probes that don't need it. At return time the args
reference is the same Ruby object as at entry, so values reflect any
in-place mutation by the method body — matching what conditions already
see and what cross-tracer convention reports for method exit scope.

Adds an integration test exercising a method probe with a capture
expression referencing arg1 against a positional argument.

Addresses review comment on PR #5845 from @chatgpt-codex-connector (3344167304).

Co-Authored-By: Claude <noreply@anthropic.com>
@p-datadog p-datadog force-pushed the di-capture-expressions branch from 8b413eb to 1bf6c26 Compare June 9, 2026 14:01
@p-datadog p-datadog marked this pull request as ready for review June 15, 2026 22:13
@p-datadog p-datadog requested review from a team as code owners June 15, 2026 22:13
@p-datadog p-datadog requested a review from Copilot June 15, 2026 22:13
@p-datadog p-datadog marked this pull request as draft June 15, 2026 22:16

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 35 out of 35 changed files in this pull request and generated 5 comments.

Comment thread lib/datadog/di/probe_notification_builder.rb Outdated
Comment thread lib/datadog/di/probe_notification_builder.rb Outdated
Comment thread lib/datadog/di/probe_notification_builder.rb Outdated
Comment thread lib/datadog/di/capture_expression_evaluator.rb Outdated
Comment thread lib/datadog/di/capture_expression_evaluator.rb Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7cf491474a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread lib/datadog/di/instrumenter.rb Outdated
p-ddsign and others added 4 commits June 29, 2026 17:22
Adds capture-expression support to Ruby log probes.

* Parse the `captureExpressions` field in remote-config LOG_PROBE payloads.
* Evaluate each expression against the probe scope at fire time.
* Emit a `captureExpressions` block in the snapshot under the same
  `captures.{lines.<n>|entry|return}` key that full snapshots use.
* Per-fire time budget controlled by
  `DD_DYNAMIC_INSTRUMENTATION_CAPTURE_TIMEOUT_MS` (default 200).
  Remaining expressions emit a stub `{notCapturedReason: "timeout"}`.
* Snapshot wins at fire time when both `captureSnapshot` and
  `captureExpressions` are set on the same probe (matches Python/Java/Go).
* Per-expression evaluation errors are added to `evaluationErrors` with
  the failed expression's name; the failed key is omitted from the
  `captureExpressions` block.
* Capture-expression probes default to the snapshot rate-limit bucket
  (1/sec); existing `sampling.snapshotsPerSecond` overrides this.

Tests cover parsing (including the backend name pattern
`^[a-zA-Z0-9_?]+$`), the rate-limit fallback, per-expression depth
override, time-budget exhaustion, and the success/error/timeout output
shapes.

Known limitation: per-expression `maxLength` and `maxCollectionSize`
are not yet honored end-to-end; the snapshot serializer reads those
two limits from the DI settings directly. Per-expression
`maxReferenceDepth` and `maxFieldCount` work.
- Thread max_length and max_collection_size kwargs through Serializer#serialize_value
  and its recursive calls, so per-expression CaptureLimits overrides for all four
  fields work end-to-end (previously only depth and attribute_count were honored).
- Add Steep signatures for the new ProbeBuilder helpers, the
  capture_expression_evaluator accessor on ProbeNotificationBuilder, and the new
  length/collection_size kwargs on Serializer#serialize_value.
- Remove the 'known limitation' note from docs/DynamicInstrumentation.md.
…_SERIALIZE

The prior commit added DD_DYNAMIC_INSTRUMENTATION_CAPTURE_TIMEOUT_MS as
a new key, which would require central Configuration Registry
coordination and broke validate_supported_configurations_v2_local_file.

Switch to the existing DD_DYNAMIC_INSTRUMENTATION_MAX_TIME_TO_SERIALIZE
env var that .NET already declares (and which is already centrally
registered). Its 200 ms default matches our chosen budget; its
semantics extend naturally to whole-snapshot serialization if Ruby
later applies the budget more broadly.

* Rename the setting from capture_expression_timeout_ms to
  max_time_to_serialize_ms.
* Drop the new DD_DYNAMIC_INSTRUMENTATION_CAPTURE_TIMEOUT_MS entry
  from supported-configurations.json and supported_configurations.rb;
  add the existing DD_DYNAMIC_INSTRUMENTATION_MAX_TIME_TO_SERIALIZE.
* Update docs/DynamicInstrumentation.md to reference the .NET-aligned
  env var.
* Update the capture-expression evaluator and spec to use the new
  setting name.
…d length

The CaptureLimits.resolve doc comment promises a per-field fallback chain
of expr_limits -> probe -> settings, but for collection_size and length the
code went straight from the expression-level override to settings, skipping
the probe-level overrides parsed into probe.max_capture_collection_size /
probe.max_capture_string_length. That meant a probe-level capture.maxCollectionSize
or capture.maxLength from remote config was silently ignored unless repeated
on every expression.

Adds the probe-level step to both fallback chains so the resolver matches
its documented contract. The serializer already accepts length: and collection_size:
keyword arguments, so the fix flows end-to-end.

Addresses review comments on PR #5845 from @Copilot (3344166066) and
@chatgpt-codex-connector (3344167297).

Co-Authored-By: Claude <noreply@anthropic.com>
This PR added two Datadog.logger.debug calls in ProbeBuilder (master had
none), reaching for the global logger from module_function code — a
Component Pattern violation (component code must use injected dependencies,
not globals). Both log paths originate from build_from_remote_config
(the second via parse_evaluate_at), and the sole production caller,
Component#parse_probe_spec_and_notify, already has the injected DI::Logger
facade in scope.

- probe_builder.rb: build_from_remote_config now takes a required
  `logger:` kwarg and threads it into parse_evaluate_at; both
  Datadog.logger.debug calls become logger.debug
- component.rb: pass the component's injected logger at the call site
- probe_builder.rbs: update the two signatures
- specs: inject a logger double at every build_from_remote_config call
  site; the two probe_builder_spec assertions now expect on the injected
  logger rather than the Datadog.logger global

Verified: rspec (probe_builder, component, integration instrumentation +
notification builder, remote = 137, 0 failures), StandardRB clean,
Steep clean.
@p-datadog p-datadog marked this pull request as ready for review July 2, 2026 00:59
@p-datadog p-datadog requested a review from Copilot July 2, 2026 00:59
@p-datadog

Copy link
Copy Markdown
Member Author

@codex review

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 38 out of 38 changed files in this pull request and generated no new comments.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. You're on a roll.

Reviewed commit: b5cd07b8f3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

# Conflicts:
#	lib/datadog/di/instrumenter.rb
#	spec/datadog/di/probe_spec.rb
@jeff-morgan-dd jeff-morgan-dd self-assigned this Jul 2, 2026
p-ddsign and others added 9 commits July 2, 2026 19:25
Replace em-dash and arrow characters introduced by this PR's comments
with ASCII equivalents (-- and ->).
Replace string-name double("settings", ...) with
instance_double(Datadog::Core::Configuration::Settings, ...) in the
CaptureLimits.resolve and Probe#snapshot_serializer_limits specs so the
settings interface is verified.
The backend permits duplicate captureExpressions names and forwards them
unchanged. ProbeBuilder now collapses entries sharing a name, keeping the
last occurrence, so each name is evaluated once instead of being evaluated
and then overwritten when the snapshot is serialized. The collapse is
logged at debug.
Strips all added non-pragma comment lines (374 across lib and spec) from the
capture-expressions change. Functional pragmas (frozen_string_literal,
standard:disable, steep:ignore) are kept. Code and tests unchanged.
Comment thread lib/datadog/di/probe.rb
Comment thread lib/datadog/di/instrumenter.rb Outdated
Comment thread lib/datadog/di/instrumenter.rb Outdated
Comment thread lib/datadog/di/probe_notification_builder.rb Outdated
Comment thread sig/datadog/di/context.rbs Outdated
p-ddsign added 2 commits July 14, 2026 09:55
…n Probe

Address review comments (extract conditions as methods on `probe`;
`snapshot_serializer_limits` flagged as a delegation candidate):

- lib/datadog/di/probe.rb: add `evaluate_at_entry?`, `capture_expressions_only?`,
  and `capture_entry_expressions?`; `snapshot_serializer_limits` now delegates to
  `CaptureLimits.resolve`, removing the duplicated probe->settings fallback chain.
- lib/datadog/di/instrumenter.rb (535, 613): call the new predicates.
- lib/datadog/di/probe_notification_builder.rb (112): use `evaluate_at_entry?`.
- sig/datadog/di/probe.rbs: signatures for the three predicates.
- spec/datadog/di/probe_spec.rb: coverage for the three predicates.

Behavior-preserving: the predicate bodies are the identical boolean expressions,
and CaptureLimits.resolve(expr_limits: nil, ...) returns the identical limits hash.
Verified locally: probe_spec (44), probe_notification_builder_spec (30),
instrumenter_spec (112) all pass, 0 failures.
Address review comment (use `any` instead of `untyped`): switch the untyped
values introduced by this PR in the DI signature subtree to the repo's `any`
alias (`type any = untyped`, sig/datadog.rbs), documenting "accepts any type".

Scoped to the signatures this PR added:
- sig/datadog/di/context.rbs: entry_capture_expressions (ivar, ctor param, reader)
- sig/datadog/di/serializer.rbs: serialize_args, serialize_vars, serialize_value
- sig/datadog/di/probe_builder.rbs: build_from_remote_config, build_capture_expressions,
  build_capture_limits
- sig/datadog/di/capture_expression_evaluator.rbs: evaluate

`any` is identical to `untyped` (alias), so no type-checking behavior changes.
Remaining pre-existing DI/symdb untyped are converted in a follow-up PR.
@TonyCTHsu TonyCTHsu enabled auto-merge July 14, 2026 17:50
@TonyCTHsu TonyCTHsu disabled auto-merge July 14, 2026 18:24
@TonyCTHsu TonyCTHsu merged commit 97cc6f6 into master Jul 14, 2026
589 checks passed
@TonyCTHsu TonyCTHsu deleted the di-capture-expressions branch July 14, 2026 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI Generated Largely based on code generated by an AI or LLM. This label is the same across all dd-trace-* repos core Involves Datadog core libraries debugger Live Debugger (+Dynamic Instrumentation, +Symbol Database)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants